The Static Analysis tool Coverity added that the following code is dead: >>if (foreignKeysWereEnabled) { >> stmtSQL.AppendLiteral("ON"); >>} else { >> stmtSQL.AppendLiteral("OFF"); >>} This code executes only if |changedForeignKeys | is true, when this code gets executed |foreignKeysWereEnabled| cannot be true because of the previous statements: >> if (foreignKeysWereEnabled) { >> changedForeignKeys = false; >> }

Review commit: https://reviewboard.mozilla.org/r/40053/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/40053/

We don't need two bools, don't we ?

Comment on attachment 8730630 [details] MozReview Request: Bug 1256593 - remvoed dead code. r?sicking Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40053/diff/1-2/

Comment on attachment 8730630 [details] MozReview Request: Bug 1256593 - remvoed dead code. r?sicking Jan, can you take this one? If this code really can't execute, we should check if that's due to a typo, or if the behavior is actually correct.

Comment on attachment 8730630 [details] MozReview Request: Bug 1256593 - remvoed dead code. r?sicking https://reviewboard.mozilla.org/r/40053/#review37401 I believe this patch is correct. We just need to temporarily enable foreign keys for the foreign keys check and if they are already enabled we don't neet to enable/disable them.